-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use time_ago_in_words
instead of actionview
#17
Conversation
Change time_ago_in_words implementation. Using this smaller gem will make it easier to support `ten_years_rails` with older versions of Ruby.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this fix but I'm concern that the gem we're now using is no longer maintained. Your PR gave me an idea that might support both old and new codebases well. Let me know what you think!
ten_years_rails.gemspec
Outdated
@@ -20,8 +20,7 @@ Gem::Specification.new do |spec| | |||
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } | |||
spec.require_paths = ["lib"] | |||
|
|||
spec.add_dependency "actionview" | |||
spec.add_dependency "activesupport" | |||
spec.add_dependency "time_ago_in_words" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gem appears to no longer be maintained. What if we approached this as a progressive enhancement? If actionview
is available, use it. If not, just print the timestamp.
It might look something like this:
begin
require "action_view"
rescue LoadError
# actionview gem not installed, that's okay.
end
module DateHelpers
if defined?(ActionView::Helpers::DateHelper)
include ActionView::Helpers::DateHelper
def time_ago_in_words(time)
"#{super(time)} ago"
end
else
# When ActionView isn't available, print ISO8601 formatted string
def time_ago_in_words(time)
time.strftime("%Y-%m-%d")
end
end
end
class Foo
include DateHelpers
def test(time)
time_ago_in_words(time)
end
end
puts Foo.new.test(Time.new(2010, 1, 1))
This would allow us to remove ActionView from the Gemfile while maintaining the pretty printing for those who have ActionView.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the progressive approach. I'll tweak my PR to take that into account.
Also, add `timecop` dependency to test time sensitive specs
This will make it a little easier to add test coverage for `GemInfo` and `BundleReport` Also, I removed unused methods and made `time_ago_in_words` a little smarter so that it doesn't depend on `ActionView` helpers
e90f0af
to
94d5ed9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jnraine I just updated this PR.
In the process of improving BundleReport
I thought it was best to move classes to the lib
directory. This made it easier for me to add a spec for the GemInfo#age
method.
Please check it out.
Thanks!
Hey @etagwerker, |
Hey @jnraine,
This PR removes the
actionview
dependency and replaces it withtime_ago_in_words
I believe this will make it easier to support this gem with older versions of ruby.
What do you think?
Thanks!